Skip to content

feature: IC preview on hover#363

Merged
darktorres merged 2 commits into
masterfrom
ic_preview
May 6, 2026
Merged

feature: IC preview on hover#363
darktorres merged 2 commits into
masterfrom
ic_preview

Conversation

@joao-zip
Copy link
Copy Markdown
Collaborator

The preview will look like this:

image

@joao-zip joao-zip added this to the v5.1 milestone Apr 28, 2026
@joao-zip joao-zip requested a review from darktorres April 28, 2026 23:33
@joao-zip joao-zip self-assigned this Apr 28, 2026
@joao-zip joao-zip linked an issue Apr 28, 2026 that may be closed by this pull request
@joao-zip joao-zip moved this to In review in wiRedPanda Apr 29, 2026
Copy link
Copy Markdown
Member

@darktorres darktorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two critical dangling-pointer issues must be fixed before merge. All other findings are inline.

Optional improvement — shallow-load the blob for a faithful preview

The preview currently renders from m_internalElements (the simulation graph). loadBoundaryElement() replaces every Input/Output element with a proxy Node, so the preview shows unlabelled Nodes at every boundary instead of the actual buttons, switches, and LEDs the user placed.

A shallow parse of the IC blob/file would fix this without any format changes. The format is sequential (preamble → elements → connections), the blob registry lives in the preamble metadata, and IC deserialization is fully encapsulated in IC::load(). A shallow variant skips deserializeAndLoad() for nested ICs, reads only visual properties, and produces a correct one-level rendering. Result is cached via m_previewValid, so the parse cost is paid only once per hover session.

Comment thread App/Element/ICPreviewPopup.h Outdated
Comment thread App/Element/IC.cpp
Comment thread App/Element/IC.cpp Outdated
Comment thread App/Element/ICPreviewPopup.cpp Outdated
Comment thread App/Element/ICPreviewPopup.cpp
Comment thread App/Element/IC.cpp Outdated
Comment thread App/Element/IC.cpp Outdated
Comment thread App/Element/IC.cpp Outdated
Comment thread App/Element/IC.cpp
Comment thread CMakeLists.txt Outdated
@darktorres darktorres force-pushed the ic_preview branch 3 times, most recently from a6f8e35 to e0abead Compare May 6, 2026 14:47
- Remove consecutive blank lines (GraphicElement.h)
- Add missing trailing newlines (TestDanglingPointer.cpp/h)
- Fix include ordering (GraphicsView.cpp, TestLanguageManager.cpp)
@darktorres darktorres force-pushed the ic_preview branch 3 times, most recently from b8e7448 to 54d1b8d Compare May 6, 2026 18:01
Adds a hover-triggered preview popup so users can identify the contents
of an IC without double-clicking to open its sub-circuit.  Hovering an
IC for one second shows a tooltip-like popup with a rendered miniature
of the IC's internal circuit, dismissed on a 300 ms delay when the
cursor leaves both the IC and the popup itself.

# Designed circuit, not simulation graph

The preview is rendered from the designed circuit — the actual buttons,
switches, LEDs, and displays the user placed — rather than the post-load
simulation graph in which boundary Input/Output elements are stripped
and replaced with proxy `Node` junctions during `IC::loadBoundaryElement()`.
The pixmap is captured eagerly at the top of `IC::processLoadedItems()`,
while the freshly-deserialized `items` list still contains the original
boundary elements with their connections and ports intact, then the
substitution loop runs as before.  The fix is strictly cheaper than a
shallow re-parse of the IC blob/file: one render at load time, no
re-deserialization, no format changes.

Reload is automatic.  `ICRegistry`'s `QFileSystemWatcher` triggers
`IC::loadFile()` on every IC instance referencing a file when that file
changes, which re-enters `processLoadedItems()` and regenerates the
pixmap.  Embedded ICs follow the same path via `loadFromBlob()`.

# Popup ownership

The popup is owned by `MainWindow` as a Qt child (`new ICPreviewPopup(this)`
in the ctor).  Qt's parent/child cleanup destroys it before `~QWidget`
finishes, well before `~QApplication` runs — no singletons, no
`qAddPostRoutine`, no lazy creation.  `MainWindow` holds it as a
`QPointer` so accesses during teardown are safe regardless of child
destruction order.  `IC` reaches it through a small file-scope helper
that walks Application → MainWindow → popup with defensive guards on
each link, but during normal operation the chain is always alive.

# UX details

- The popup positions itself near the cursor with a 16 px offset and is
  clamped to the available screen geometry so it never spills off-screen.
- `hoverMoveEvent` updates the pending position while the show timer is
  running, so the popup appears near the *current* cursor when the
  delay elapses, not at the original entry point.
- Cursor entering the popup itself cancels any scheduled hide, letting
  users move onto the preview to read it.
- Double-clicking an IC (which opens its sub-circuit in a new tab) hides
  the popup immediately so it doesn't overlap the tab transition.
- The popup chrome is a translucent dark window (OS-frameless:
  `Qt::ToolTip | Qt::FramelessWindowHint`) with a custom-drawn 6 px-radius
  border; the preview pixmap sits inside an inset frame (1 px light
  border, 6 px padding, near-black inset background) so the rendered
  circuit reads as a framed image rather than floating content.

# Performance and edge cases

- Circuits exceeding `MaxElementCount = 500` are skipped — the preview
  pixmap is left null, and `executeShow()` hides instead of showing.
  500 was chosen empirically: rendering above this size exceeds 16 ms
  on a mid-range laptop, making the 1-second hover delay feel laggy.
- Empty circuits are skipped the same way.
- Pixmap rendering temporarily borrows the `items` into a throwaway
  `QGraphicsScene`, calls `render()`, then removes them.  A `qScopeGuard`
  ensures the borrow is reversed even if `render()` throws — IC owns the
  items, so a stuck borrow at scene destruction would be a double-free.
- The destination `QPixmap` is explicitly filled with the background
  brush before `render()`.  Without the fill, `tempScene.render()`'s
  source→target affine can leave a 1-pixel sliver unpainted at the
  right/bottom edge, exposing uninitialized pixmap memory (commonly
  white on Windows).

# Lifetime safety

`ICPreviewPopup::m_pendingIC` is a `QPointer<IC>` so it auto-nulls if
the IC is destroyed while a show or hide timer is pending.  `IC::~IC()`
additionally cancels and hides the popup if it was about to display
*this* IC, so the user never sees stale content from a destroyed IC.
The cached pixmap held by the popup's `QLabel` is implicitly shared
with the IC's `m_previewPixmap`, so the displayed image survives the
IC's destruction even though the IC pointer becomes null.
@darktorres darktorres merged commit 14e4f46 into master May 6, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in wiRedPanda May 6, 2026
@darktorres darktorres deleted the ic_preview branch May 6, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Show IC subcircuit preview on hover

2 participants